Skip to content

fix(memory): source forwarded env below memtrack's capability boundary#439

Merged
not-matthias merged 1 commit into
mainfrom
cod-3076-mozilla-slack-message
Jul 7, 2026
Merged

fix(memory): source forwarded env below memtrack's capability boundary#439
not-matthias merged 1 commit into
mainfrom
cod-3076-mozilla-slack-message

Conversation

@not-matthias

@not-matthias not-matthias commented Jul 7, 2026

Copy link
Copy Markdown
Member

Root cause

codspeed-memtrack holds file capabilities (cap_bpf, cap_perfmon, cap_sys_admin). A non-root user executing a capability binary enters glibc secure-execution mode (AT_SECURE=1), which strips LD_* variables from the environment before main().

The memory executor sourced the forwarded environment around memtrack:

bash -c "source ENV && memtrack track … '<bench>'"   ← LD_LIBRARY_PATH restored here
└─ codspeed-memtrack                                 ← cap binary: glibc strips LD_* again
   └─ bash -c <bench>                                ← inherits stripped env ❌

So LD_LIBRARY_PATH was restored and then stripped a second time, before the benchmark memtrack spawns could inherit it.

Walltime is unaffected because it layers privilege escalation (sudo/perf/systemd scope) outside the env re-source, keeping source at the innermost layer.

Fix

Source the env inside the command memtrack runs via its own bash -c, below the capability boundary:

codspeed-memtrack track … "source ENV && <bench>"
└─ codspeed-memtrack           ← strips LD_* (no longer matters)
   └─ bash -c "source ENV && <bench>"   ← re-sources below the cap binary
      └─ <bench>                        ← sees LD_LIBRARY_PATH ✅
  • Add prefix_command_with_env for the raw source <file> && <cmd> snippet (no outer bash -c) and reuse it from wrap_with_env.
  • MemoryExecutor::build_memtrack_command now prefixes the bench command and passes it as memtrack's argument; the outer bash wrapper is gone.

Test

test_memory_executor_forwards_ld_library_path (Linux + GITHUB_ACTIONS gated) exercises this end-to-end. The existing ..._forwards_path test never caught it because PATH is not an LD_* variable.

Note

If the memory test suite is ever run as root, memtrack's caps add nothing, secure-exec isn't triggered, and the test would pass even with the bug present, same limitation as the real-world scenario, which only affects the non-root runner user.

Verification

Reproduced the mechanism with a minimal capability binary that runs its argument via bash -c (memtrack's exact structure), as a non-root user:

nesting benchmark sees LD_LIBRARY_PATH
old (source around cap binary) empty ❌
new (source inside bench) /custom/lib

@codspeed-hq

codspeed-hq Bot commented Jul 7, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

⚠️ Unknown Walltime execution environment detected

Using the Walltime instrument on standard Hosted Runners will lead to inconsistent data.

For the most accurate results, we recommend using CodSpeed Macro Runners: bare-metal machines fine-tuned for performance measurement consistency.

✅ 7 untouched benchmarks


Comparing cod-3076-mozilla-slack-message (6bf0d30) with main (7ce2c98)

Open in CodSpeed

@greptile-apps

greptile-apps Bot commented Jul 7, 2026

Copy link
Copy Markdown

Greptile Summary

This PR fixes memory benchmark environment forwarding across the memtrack capability boundary. The main changes are:

  • Adds a raw helper for source <env-file> && <command> snippets.
  • Moves env sourcing into the benchmark command passed to memtrack.
  • Adds a Linux CI regression test for LD_LIBRARY_PATH forwarding.

Confidence Score: 5/5

This looks safe to merge.

  • No blocking issues found in the changed code.

Important Files Changed

Filename Overview
src/executor/helpers/run_with_env.rs Adds a reusable helper for env-prefixed shell snippets while keeping the existing wrapper behavior.
src/executor/memory/executor.rs Moves forwarded env sourcing below memtrack so LD_* values reach the benchmark process.
src/executor/tests.rs Adds coverage for memory benchmark forwarding of LD_LIBRARY_PATH.

Reviews (3): Last reviewed commit: "fix(memory): source forwarded env below ..." | Re-trigger Greptile

@not-matthias not-matthias force-pushed the cod-3076-mozilla-slack-message branch 2 times, most recently from ec0a5c8 to 1f71d95 Compare July 7, 2026 09:30
@not-matthias not-matthias marked this pull request as ready for review July 7, 2026 09:31

@GuillaumeLagrange GuillaumeLagrange left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OLGTM

Key question though: why did we not catch it before? Is there an integration test that we are missing?

Comment thread src/executor/helpers/run_with_env.rs Outdated
@not-matthias

not-matthias commented Jul 7, 2026

Copy link
Copy Markdown
Member Author

Key question though: why did we not catch it before? Is there an integration test that we are missing?

We had a test which tested forwarding the PATH env var, but LD_* env vars are handled differently

codspeed-memtrack holds file capabilities, so a non-root user executing it
enters glibc secure-execution mode (AT_SECURE=1), which strips LD_* variables
from the environment before main(). The memory executor sourced the forwarded
environment around memtrack, so LD_LIBRARY_PATH was restored and then stripped
again before the spawned benchmark could inherit it. Benchmarks relying on
LD_LIBRARY_PATH (e.g. neqo loading a locally built NSS) fell back to the system
library and panicked with UnsupportedVersion.

Source the env inside the command memtrack runs via `bash -c`, matching how
walltime already layers privilege escalation outside the env re-source. Add
prefix_command_with_env for the raw `source <file> && <cmd>` snippet and reuse
it from wrap_with_env.
@not-matthias not-matthias force-pushed the cod-3076-mozilla-slack-message branch from 1f71d95 to 6bf0d30 Compare July 7, 2026 12:02
@not-matthias not-matthias merged commit 6bf0d30 into main Jul 7, 2026
22 checks passed
@not-matthias not-matthias deleted the cod-3076-mozilla-slack-message branch July 7, 2026 12:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants